Skip to content

Fix for FORK branches with mixed outputs and unsupported field types #129636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 19, 2025
Merged

Fix for FORK branches with mixed outputs and unsupported field types #129636

merged 4 commits into from
Jun 19, 2025

Conversation

@ioanatia
Copy link
Contributor

@ioanatia ioanatia commented Jun 18, 2025 •
edited
Loading

related: #121950

Fixes an issue when FORK branches have different outputs which contain unsupported fields.

When a FORK branch is missing an column X that is present in another branch, we add an EVAL X = null to make sure all branches have the same output. We also align the attributes with a KEEP on each FORK branch.

In the case where the indices we query have fields with mapping types that are not supported and the FORK branches have different outputs, we can get failures such as this:

FROM my-index
| FORK (DROP x) // x is missing
       (WHERE true) // x is UNSUPPORTED

{
  "error": {
    "root_cause": [
      {
        "type": "verification_exception",
        "reason": """Found 1 problem
line 3:12: EVAL does not support type [unsupported] as the return data type of expression [FROM my-index]"""
      }
    ],
    "type": "verification_exception",
    "reason": """Found 1 problem
line 3:12: EVAL does not support type [unsupported] as the return data type of expression [FROM my-index]"""
  },
  "status": 400
}

This is because we add a EVAL x = null to the first branch, with x having an UNSUPPORTED data type.

To fix this, we needed to make sure we don't break the current behaviour where if two branches have the same column name but with different types, we need to return a validation error.
For example, here we return a validation error and we need to preserve this behaviour:

FROM my-index
| FORK (WHERE true) // x is UNSUPPORTED 
        (stats x = COUNT(*)) // x is LONG

returns:

{
  "error": {
    "root_cause": [
      {
        "type": "verification_exception",
        "reason": """Found 1 problem
line 2:5: Column [x] has conflicting data types in FORK branches: [UNSUPPORTED] and [LONG]"""
      }
    ],
    "type": "verification_exception",
    "reason": """Found 1 problem
line 2:5: Column [x] has conflicting data types in FORK branches: [UNSUPPORTED] and [LONG]"""
  },
  "status": 400
}

@ioanatia ioanatia added >non-issue :Search Relevance/Search Catch all for Search Relevance v9.1.0 Team:Search - Relevance The Search organization Search Relevance team labels Jun 18, 2025
@ioanatia ioanatia mentioned this pull request Jun 18, 2025
Closed
23 tasks
@ioanatia ioanatia marked this pull request as ready for review June 19, 2025 07:57
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Jun 19, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jun 19, 2025

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine elasticsearchmachine removed the Team:Search - Relevance The Search organization Search Relevance team label Jun 19, 2025
ChrisHegarty
ChrisHegarty approved these changes Jun 19, 2025
View reviewed changes
Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tteofili
tteofili approved these changes Jun 19, 2025
View reviewed changes
Copy link
Contributor

@tteofili tteofili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ioanatia ioanatia enabled auto-merge (squash) June 19, 2025 09:38
@ioanatia ioanatia disabled auto-merge June 19, 2025 09:39
@ioanatia ioanatia merged commit c173522 into elastic:main Jun 19, 2025
27 checks passed
kderusso pushed a commit to kderusso/elasticsearch that referenced this pull request Jun 23, 2025
@ioanatia @kderusso
Fix for FORK branches with mixed outputs and unsupported field types (e…
d5a273f 
…lastic#129636)
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jun 25, 2025
@ioanatia @mridula-s109
Fix for FORK branches with mixed outputs and unsupported field types (e…
8f24b1d 
…lastic#129636)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tteofili tteofili tteofili approved these changes

@ChrisHegarty ChrisHegarty ChrisHegarty approved these changes

@carlosdelest carlosdelest Awaiting requested review from carlosdelest

Assignees

No one assigned

Labels

>non-issue :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ioanatia @elasticsearchmachine @tteofili @ChrisHegarty